fix: LightGBM Ensure non-duplicate column names - #2508

Related Issues/PRs

Fixes #2242

Fixes the LightGBM training failure Feature (Column_) appears more than one time.

What changes are proposed in this pull request?

LightGBM rejects a Dataset whose feature names repeat. Spark can surface repeated names through the AttributeGroup metadata of the features column (and a user can pass repeated slotNames directly), so training failed in the native LGBM_DatasetSetFeatureNames call with:

Dataset set feature names call failed in LightGBM with error:
Feature (Column_) appears more than one time.

Changes

• De-duplicate feature names before handing them to LightGBM. getSlotNamesWithMetadata now routes both metadata-derived names and explicit slotNames through ensureUniqueFeatureNames, which appends a numeric suffix to repeated names while preserving order (feature names are positional in LightGBM).
• Name the streaming reference Dataset. createReferenceDatasetFromSample now accepts the feature names and applies them, so the reference Dataset carries the same names as the per-partition Datasets.
• Split createReferenceDatasetFromSample into createDatasetFromSamples, setFeatureNamesIfProvided, and serializeAndCleanup.
• Validate slot names unconditionally. validateSlotNames previously ran only when the features column carried AttributeGroup metadata, so explicit slotNames containing " , : [ ] { } were never checked. See the behavior-change note below.
• Validate slot names before the first native call that consumes them. Naming the reference Dataset introduces an LGBM_DatasetSetFeatureNames call inside calculateRowStatistics, which runs earlier in trainOneDataBatch than validateSlotNames did. validateSlotNames is now called as soon as featuresSchema is resolved.

Correctness notes

Generated names cannot collide with later original names. A dedup pass that only remembers the names it has already emitted is not sufficient. For ["Column_", "Column_", "Column__1"] it renames the second Column_ to Column__1, which the third slot already owns, so LightGBM still fails — on the exact error this PR exists to fix. Every original name is therefore reserved up front, and generated names are reserved as they are handed out. This is covered by a dedicated test that was confirmed to fail against the naive implementation with Feature (Column__1) appears more than one time.

Uniqueness is decided on LightGBM's terms. LightGBM replaces every space in a feature name with an underscore before checking for duplicates, so "a b" and "a_b" are one feature natively while being two distinct strings in Scala. Comparing raw strings therefore still let the target error through:

Dataset set feature names call failed in LightGBM with error:
Feature (a_b) appears more than one time.

Collisions are now detected on the normalized form while the original names are still what get emitted. Confirmed empirically in both directions.

No bulk-mode retry. An earlier revision of this branch caught the failure and retried the batch with dataTransferMode=bulk. That fallback was removed because it could not work:

1. BulkPartitionTask calls setFeatureNames with the same still-duplicated names, so the retry fails identically. This is the decisive point and holds regardless of which native call reports the duplicate.
2. Its guard also required the message to contain dataset create. That matches the call site in #2242 (Dataset create from samples), but on current master the duplicate is reported by LGBM_DatasetSetFeatureNames, whose component string is Dataset set feature names, so the guard no longer matches the failure it was written for. Confirmed empirically; the failure text is quoted above.

It also mutated the estimator's dataTransferMode param mid-fit, which is visible to concurrent callers and to the model built during the retry. Deduplicating the names up front removes the need for a fallback entirely.

Native handle cleanup. createDatasetFromSamples frees its voidpp handle in a finally, matching deserializeReferenceDataset. The pre-existing code allocated two handles and freed only the unused one, leaking the one it actually used on every streaming fit().

The reference Dataset itself is now also freed in a finally spanning both naming and serialization. Previously it was freed only on the success path, so the naming call this PR adds would have leaked it whenever a name was rejected. That free is intentionally not routed through LightGBMUtils.validate, because throwing from the finally would mask the original failure.

Backslash is not rejected. The invalid-slot-name message listed \ as a disallowed character, but the regex never matched it and LightGBM does not reject it either — CheckAllowedJSON rejects exactly " , : [ ] { }. The message now matches both the regex and the native behavior. The regex is deliberately unchanged; adding \ would reject names LightGBM accepts.

Feature-name count guard. LGBM_DatasetSetFeatureNames reads numCols entries from the array, so a shorter array would be an out-of-bounds native read. When the counts disagree the names are skipped with a warning, which is the behavior prior to this change.

Validation must precede the native call. LightGBM rejects " , : [ ] { } in feature names itself, but only with Do not support special JSON characters in feature name. — it does not say which column is at fault. Because naming the reference Dataset moves the first native feature-name call earlier in fit, validating afterwards let that opaque error win the race and broke the existing Verify LightGBM Regressor with bad column names fails early test. Validating up front restores the actionable IllegalArgumentException, and the test now fails in 0.3s instead of after the sampling pass — which is what "fails early" is asserting.

Behavior changes

• Repeated feature names are now renamed rather than fatal. A warning lists the names that were renamed.
• validateSlotNames now also validates explicitly supplied slotNames. A job that passed slotNames containing " , : [ ] { } while the features column had no attribute metadata previously reached LightGBM unchecked and now fails fast with the existing, actionable IllegalArgumentException.

How is this patch tested?

• I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

Added to VerifyLightGBMCommon:

Validated locally on Java 11 against the full CI dataset set: VerifyLightGBMCommon 9/9, VerifyLightGBMRegressorStream and VerifyLightGBMRankerStream 26/26 (including Verify LightGBM Regressor with bad column names fails early), VerifyLightGBMRegressorBulk, VerifyLightGBMRankerBulk, VerifyLightGBMClassifierStreamOnly, NetworkManagerSuite and TrainUtilsSuite 52/52, and lightgbm/scalastyle plus lightgbm/Test/scalastyle report 0 errors.

An earlier revision of this branch also set setUseBarrierExecutionMode(true) in LightGBMRankerTestData and LightGBMRegressorTestData. Those are EstimatorFuzzing base classes, so the flag applied to every ranker and regressor test rather than to anything this PR changes; it has been reverted.

Rebased onto current master (68 commits, including #2595 and #2578) with no conflicts.

Does this PR change any dependencies?

• No. You can skip this section.

Does this PR add a new feature? If so, have you added samples on website?

• No. You can skip this section.